Do not swallow every reply conflict as dedup

When the original dispatch is cancelled or failed instead of answered (for example, offline-session cancellation or stuck-task recovery flips a delegated task while Claude is still composing), /reply now also produces a 409 because the server resolves non-delegated tasks and the atomic claim fails. This branch treats every 409 as a benign sibling-session dedup, and index.ts then reports “Already answered” without posting any comment, so a valid response can be silently dropped. Please distinguish the completed/dedup case from cancelled/failed/not-delegated conflicts before suppressing the error.

Useful? React with 👍 / 👎.

Confirmed and fixed in 96bb8ac. The claim fails identically in both cases, which is exactly why the client could not tell them apart: claim_delegated_task returns nil for any task that is not delegated, so a dispatch cancelled by an offline session, failed, or flipped by stuck-task recovery while Claude was composing produced the same bare 409 as the sibling-session dedup. The dedup case has an answer already posted; the others have none, and the text in hand is the only copy of it — so index.ts reporting "Already answered" and posting nothing lost a real reply, silently.

The server now names the cause rather than leaving the client to guess:

• TaskClaimService#conflict_reason → already_completed when the task is done (a sibling won the claim, or the agent's own job completed it — either way the dispatch was answered and this reply is a duplicate), not_delegated for every other status.
• reply renders it beside the existing message: { error: ..., reason: ... }, still 409.

The client suppresses on the marker only. isBenignReplyDedup parses the body and requires reason === "already_completed"; anything else — not_delegated, an unparseable body, or a response with no reason at all — is thrown as Reply failed (409). That last one is the back-compat call: an older server sending the unmarked body gets the pre-dedup behaviour (a visible error) rather than the silent drop. Surfacing a conflict the model can act on is recoverable; dropping the user's answer is not.

CONFLICT_UNKNOWN_TASK is deliberately not a third reason. resolve_reply_agent resolves the agent from the task and 403s if it is not this caller's on this topic, so by the time the conflict branch runs the task always exists — a find_by miss in conflict_reason would be unreachable, and it folds into not_delegated (surface it) as the safe default.

Tests pin both directions. Ruby: an answered dispatch replied to twice returns already_completed; cancelled, failed and pending tasks each return not_delegated with an explicit "suppressing this 409 loses the reply" assertion. Making conflict_reason unconditionally return already_completed fails that second test. TypeScript: isBenignReplyDedup over the dedup body, the cancelled body, a legacy no-reason body, and non-JSON.

agents_controller_test 74 runs / 340 assertions / 0 failures; plugin suite 32/32; tsc --noEmit and rubocop clean.

Only mark conflicts benign after a reply exists

When the first sibling has claimed a delegated task, claim sets the task to done before comment.save/finalize links the reply; if that worker crashes or the save later fails (for example blank/invalid text, after which the controller restores the task to delegated), a concurrent valid reply can hit this method during the done-without-reply window and receive reason: "already_completed". The plugin now suppresses exactly that reason, so the valid answer is dropped even though no comment was ever created; base the benign reason on an actual linked reply (or otherwise wait for finalization), not status alone. Fresh evidence here is the new status-only already_completed marker.

Useful? React with 👍 / 👎.

Confirmed and fixed in 03f5a1d. status == "done" was standing in for "the dispatch was answered", and it is not the same claim: #claim flips the task to done, and the controller saves the comment after that returns. Inside that window the row reads completed with nothing posted behind it — and the window can end in a rollback (comment.save false → the controller restores the task to delegated) or never end at all if the worker dies. A concurrent /reply arriving there was told already_completed, so the client suppressed it and dropped the only copy of an answer nobody else had written.

The linked reply comment is the proof, since #finalize is what creates the link (comment.update_column(:task_id, task.id) → Task#reply_comment):

return CONFLICT_NOT_DELEGATED unless task&.status == "done"
task.reply_comment.present? ? CONFLICT_ALREADY_COMPLETED : CONFLICT_CLAIMED_WITHOUT_REPLY

A third reason rather than folding it into not_delegated: a claimed-but-unanswered task really is done and really did leave delegated, so reporting not_delegated would put a false statement in the operator-facing half of the payload for the sake of the machine-readable half. The client keys on the one explicit already_completed marker, so a new value is non-benign by construction and needs no client change to be safe — but the test pins it anyway, because "unknown reasons happen to fall through to safe" is not something the next edit should be free to break.

One deliberate consequence: two replies that genuinely race — the second arriving in the microseconds between the winner's claim and its save — now surface a conflict instead of staying quiet, and the caller sees an error for a duplicate. That is the trade this direction has to make. Nothing at 409 time can distinguish "the winner is mid-save" from "the winner died mid-save"; only one of those two mistakes loses a message, and a retry of the spurious one converges (by then the reply is linked, and the second attempt is suppressed as dedup).

Tests both ways: a task set to done with no linked comment — exactly what #claim leaves behind — now answers claimed_without_reply, and reverting the predicate to the status check fails it with already_completed. The plugin side pins that the new reason is not benign.

agents_controller_test 75 runs / 344 assertions / 0 failures; plugin node --test 33/33; rubocop clean on both changed files.

Wait for finalization before hard-failing duplicate replies

When two same-directory Claude sessions reply at nearly the same time, the loser can reach this check after the winner's claim has committed status = done but before finalize has linked the reply comment. In that normal sibling-race window reply_comment.present? is still false, so the server returns claimed_without_reply; the client then throws instead of treating the race as benign, leaving the original 409-as-user-visible-error behavior timing-dependent even though the winning reply is about to land. Consider waiting/reloading for finalization (or otherwise making claim+link atomic) before deciding this is a hard conflict.

Useful? React with 👍 / 👎.

Confirmed and fixed in 97e3bda. You are right that the previous round made the loser's outcome timing-dependent, and the direction of that dependence is the bad one: the sibling race is the common case, and it lands in the done-without-reply window every single time. #claim commits done, then the controller runs comment.save — validations, callbacks, broadcasts — and only then does #finalize reach comment.update_column(:task_id, ...). Every winning claim passes through claimed-without-reply on its way to answered, so deciding on the first read was reporting "your answer may be the only copy" for what is almost always a duplicate that is milliseconds from landing.

The predicate is unchanged — only an actually linked comment is ever called benign, and nothing is inferred from status or elapsed time. What changed is that finalization gets a bounded chance to produce the proof before the answer is fixed:

FINALIZE_GRACE_SECONDS = 0.5
FINALIZE_GRACE_INTERVAL = 0.05

deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + FINALIZE_GRACE_SECONDS
loop do
  return true if reply_linked?(task)
  return false if Process.clock_gettime(Process::CLOCK_MONOTONIC) >= deadline

  sleep FINALIZE_GRACE_INTERVAL
end

Monotonic clock so a wall-clock adjustment cannot stretch or collapse the deadline mid-wait. The reads are uncached — that one is load-bearing rather than defensive: ActiveRecord::QueryCache wraps the request, so the identical SELECT 1 FROM comments WHERE task_id = ? would be served from the first (empty) result for the entire wait and the loop could never observe the winner's commit.

Only the window pays for it. A non-done task returns not_delegated before the wait is reached, and a reply that was already linked when the request arrived answers on the first read — both pinned by tests that assert the elapsed time stays under the grace period, so a later edit cannot quietly put the common paths behind a sleep.

I did not take the make-claim-and-link-atomic option, though it is the tighter one on paper. Doing it means holding the SELECT FOR UPDATE across comment.save, which is exactly what the current shape avoids: the long comment above #claim exists because that save fires after_update_commit work — TriggerLoopCheckJob, the stop-button broadcast — and running it inside the claim transaction puts those side effects before the commit they depend on. update_all is there specifically to keep them out. Moving the claim after the save instead reopens the duplicate-comment race the claim was written to close. Waiting buys the same observable behaviour for the loser (it blocks either way, on a lock or on a poll) without moving side effects inside a transaction.

What survives past the deadline is the case you and I both want surfaced: a claim whose worker died mid-save. It reports claimed_without_reply 500ms later than before, and nothing suppresses it.

Four tests in a new task_claim_service_test.rb, and reverting the wait to a single read fails two of them: a link that lands between the first and second read now reads as already_completed (with an assertion that it re-read at all, so a first-read decision fails even if it guesses right), and the never-finalized claim asserts both that the window was actually given and that it is bounded.

task_claim_service_test 4 runs / 22 assertions / 0 failures; agents_controller_test 75 runs / 344 assertions / 0 failures; full pre-push suite 2946 runs / 9267 assertions / 0 failures; rubocop clean on both files.